Skip to content

fix(acp): spawn prompt turn to prevent permission-gate deadlock - #6660

Merged
bug-ops merged 2 commits into
mainfrom
fix/acp-permission-gate-deadlocks
Jul 27, 2026
Merged

fix(acp): spawn prompt turn to prevent permission-gate deadlock#6660
bug-ops merged 2 commits into
mainfrom
fix/acp-permission-gate-deadlocks

Conversation

@bug-ops

@bug-ops bug-ops commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • handle_prompt awaited the entire agent turn inline inside the ACP SDK's on_receive_request callback, holding the SDK's strictly serial dispatch loop for the whole turn. Since that same loop demultiplexes inbound RPC responses, the IDE's reply to a permission-gated tool call's session/request_permission request could never route back while the loop was blocked — deadlocking indefinitely (fail-closed, not a permission bypass).
  • Fix: the turn now runs inside cx.spawn(...), responding to the session/prompt RPC request from the spawned task instead of the handler, per the SDK's documented ordering contract. This frees the dispatch loop to keep routing inbound messages, including the permission reply, while the turn runs.
  • The fix's blast radius is broader than the issue title: every send_request(...).block_task().await from a per-session spawned task (fs.rs, terminal.rs, lsp/acp_provider.rs, agent/elicitation.rs) shared the same deadlock class and is fixed at the root by this change, though only the permission.rs path is regression-tested directly.
  • The spawned task's tail is infallible by construction (matching the existing spawn_cancel_request_bridge convention in the same file): a respond/respond_with_error failure is logged and swallowed rather than propagated, since an Err returned from a spawned task tears down the entire connection per the SDK's contract, and a failed response send is a symptom of an already-closing connection, not a defect in the completed turn.

Known behavior change

"prompt already in progress" was previously unreachable from ACP because the blocked dispatch loop serialized session/prompt dispatch. It is now reachable if a client pipelines two prompts on the same session before the first completes — such a client now gets an internal_error instead of implicit queueing. Spec-conformant clients don't do this.

Follow-up (not in this PR)

do_prompt restores output_rx only on its success path; a pre-existing (not introduced here) input_tx.send failure early-return leaks it, permanently wedging the session. This is now slightly more observable given the behavior change above. Will file a dedicated follow-up issue.

Closes #6656

Test plan

  • Added permission_gated_prompt_round_trip_does_not_deadlock (approve path) and permission_gated_prompt_denial_does_not_deadlock (deny path, fail-closed) integration tests, both timeout-guarded so a regression fails fast.
  • Verified the new tests actually catch the bug — swapped in pre-fix prompt.rs, both tests failed with Elapsed(()), restored the fix.
  • cargo +nightly fmt --check clean
  • cargo clippy --profile ci --workspace --all-targets --features "desktop,ide,server,chat,pdf,scheduler,testing" -- -D warnings clean
  • cargo nextest run --config-file .github/nextest.toml --workspace --features "desktop,ide,server,chat,pdf,scheduler" --lib --bins — 15098 passed, 0 failed, 36 skipped
  • Rustdoc gate (RUSTFLAGS="-D warnings" RUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links" cargo doc --no-deps --workspace) clean
  • gitleaks protect --staged clean

bug-ops added 2 commits July 27, 2026 23:22
handle_prompt awaited the entire agent turn inline inside the ACP SDK's
on_receive_request callback, holding the SDK's strictly serial dispatch
loop for the whole turn. Since that same loop demultiplexes inbound RPC
responses, the IDE's reply to a permission-gated tool call's
session/request_permission request could never route back while the
loop was blocked on the still-running turn, deadlocking indefinitely.

Spawn the turn via cx.spawn instead, per the SDK's documented ordering
contract, and respond to the session/prompt request from inside the
spawned task. This frees the dispatch loop to keep routing inbound
messages, including the permission reply, while the turn runs. The
spawned task swallows response-send failures instead of propagating
them, since an Err returned from a spawned task tears down the entire
connection per the SDK's contract, and a failed response send is a
symptom of an already-closing connection, not a defect in this turn.

Adds integration test coverage for both the approve and deny paths of
a permission-gated prompt round trip, guarded by a timeout so a
regression fails fast instead of hanging the suite.
@github-actions github-actions Bot added documentation Improvements or additions to documentation rust Rust code changes labels Jul 27, 2026
@github-actions github-actions Bot added bug Something isn't working size/L Large PR (201-500 lines) labels Jul 27, 2026
@bug-ops
bug-ops enabled auto-merge (squash) July 27, 2026 21:26
@bug-ops
bug-ops merged commit 24bdf69 into main Jul 27, 2026
43 checks passed
@bug-ops
bug-ops deleted the fix/acp-permission-gate-deadlocks branch July 27, 2026 21:34
bug-ops added a commit that referenced this pull request Aug 16, 2026
* docs(readme): sync crate READMEs with commits since v0.22.3

Reconciles all 24 changed crate READMEs against the actual shipped
implementation for the v0.22.3..HEAD range: new subsystems (risk-chain
detection, capability scoping, plugin dependency graph, session spawn
cap), several pre-existing factual errors unrelated to this release
(inverted file-sandbox precedence, fabricated MCP config keys, wrong
anomaly-detector defaults, stale trust-level names), and terminology/
API renames that had drifted out of sync with the code.

* docs(specs): reconcile spec drift for commits since v0.22.3

Closes drift left after the skill-quarantine trust fixes (#6701,
#6702, #6706, #6707, #6713), the subagent session-wide spawn cap
(#6545), four post-ACP-2.0.0-migration bugfixes (#6660, #6665, #6672,
#6684), the mention-picker and TUI interrupt-hint updates, the
MAX_RETRY_SECS compile-time bound, the sanitizer secret-shape masking
extension, the tracing-guard-flush invariants, and the VigilGate
per-process pattern-compile fix. Updates specs/README.md's index to
match.

* docs(book): sync user docs with commits since v0.22.3

Updates the TUI keybindings and mention-picker pages for the new
Ctrl+C semantics, the inline @ mention picker, and the input
separator's busy indicator; documents the new
[tools.shell] risk_chain_window_turns config key; corrects the ACP
protocol version reference (was stale at 0.11.1); bumps the sub-agent
frontmatter breaking-change note to v0.22.4.

* fix(serve): give build_combined_deps_wires_policy_gate test a dedicated stack

cargo nextest run --features full could crash with a stack overflow
(SIGABRT) on
serve::agent_factory::tests::build_combined_deps_wires_policy_gate_through_to_session_agent.
Same defect class already fixed once in this file for issue #6699:
building a full Agent under --features full's unboxed AnyProvider
variants (Candle/Gonka/Cocoon) reaches the same VigilGate::try_new
stack depth that overflows the default 2 MiB test-thread stack in an
unoptimized build. The #6699 fix only wrapped the one test it was
filed against, leaving this one - added in PR #6007, unrelated to any
change in this release - unprotected. CI's test job never caught it
because it runs the curated feature set, not full, so the deeper
AnyProvider frames never materialize there.

Runs the test body on a dedicated 32 MiB-stack thread instead of
directly under #[tokio::test], reusing the existing
TEST_THREAD_STACK_SIZE constant.

* release: prepare v0.22.4

Bump version across the workspace, finalize the CHANGELOG.md
[0.22.4] section, refresh the README tests badge, and re-accept the
splash-screen snapshots (embed the version string).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation rust Rust code changes size/L Large PR (201-500 lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ACP permission gate deadlocks on every permission-gated tool call

1 participant